Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces documentation for the MemPalace integration with OpenCode, covering its architecture, setup, and features like real-time conversation persistence and Knowledge Graph extraction. The review feedback suggests improving the installation command for Python environment safety, correcting syntax highlighting for a JSON snippet, and fixing the configuration key from 'plugin' to 'plugins'. Additionally, a correction was made to the agent instructions to ensure the 'mempalace_kg_query' tool is used correctly without unsupported parameters to prevent validation errors.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Thanks everyone for the reviews. All suggestions have been addressed:
|
|
Also worth noting: @igorls (https://github.com/igorls) reviewed the upstream OpenCode source adapter in #1484, which takes the pull approach for retrospective ingest. My plugin is the push side (real-time capture). They're complementary — install plugin + run mempalace mine --source opencode once for full coverage. |
|
Thanks for this — the hook-based approach (chat.message + session.idle) is the right call over cron. Real-time delta sync with async mining is a clean design. ▎ uv tool install "mempalace>=3.3.5" |
|
@williamblair333 thanks for the thorough review. I've addressed all three points in v1.1.0:
The KG regex extraction was also removed — MemPalace's own mining and the model's MCP tools handle that more reliably. Plugin is at v1.1.0 on npm and the updated SKILL.md reflects all changes. Let me know if anything else is needed before merge. |
|
Maintainer review (docs-only). This is mainly a trust/policy call for the org rather than a code issue: The Minor doc fixes: the One correction to an earlier concern of mine: the "developer / creative / emotions / family / consciousness" buckets are not a design-principle violation — those map to the hall content-type classifier ( |
…terminology, reflect simplified plugin
|
@igorls thanks for the thorough maintainer review. All points addressed in the latest commit:
|
|
@igorls @williamblair333 — bumping this for visibility. The plugin has evolved significantly since May. Here's what changed:
|
- Memory injection is now the primary flow (autoInjectContext) - AGENTS.md simplified to KG-only (2 steps, no mempalace_search) - Added autoInjectContext config step (~/.mempalace/plugin-config.json) - Architecture diagram updated for transform hook flow - Added comparison table: auto-inject vs model-driven - Kept alternative model-driven path documented
|
Working through the same problem on the Claude Code side — hook-layer injection via Two things on v1.2.0–v1.3.1:
Happy to review the updated SKILL.md when pushed — just make sure @igorls's community disclaimer stays in. |
|
@williamblair333 great points, thank you.
|
|
Thanks for the update — working through the two new commits. Everything from my May and June reviews is addressed. A few new things from reading the full SKILL.md: Bug: requires:
anyBins:
- mempalace
- python3
Design note: double MCP round-trip on every response Design note: KG over-recording guidance No concerns on the MemPalace integration design itself — version pin, serialized mining, flat export, opt-in inject are all correct. The npm package trust call remains @igorls's. |
|
@williamblair333 all three points addressed in commit a5fc259:
Plugin is at v1.3.2 on npm with a fully updated README and the same KG quality guidance. |
|
@geco — all three look good.
v1.3.2 looks solid. From my side this is ready. Thanks for iterating on it this quickly. |
|
I have been using your fork for a week. It worked out great. Thanks for sharing it. Had to make changes for windows version but I think mempalace isn't windows focused at all. |
|
Thanks for iterating on this. I re-reviewed it during the agent-support merge pass. This doc/plugin direction is still valuable, especially as the real-time "push" side for OpenCode. I merged the complementary #1484 OpenCode source adapter into Before merge, I would like this updated for the current tree:
Once the doc is synchronized with the landed source adapter and current plugin behavior, this looks like a good candidate for docs/integrations merge. |
|
Status update — requesting re-review toward merge. The plugin just went through its biggest evolution (now Just pushed here: the section @igorls asked for — the SKILL.md now references What changed since the June review round:
Reviewer checklist status: @williamblair333 @igorls — mind taking another look? Happy to address anything remaining. |
igorls
left a comment
There was a problem hiding this comment.
Thanks for keeping this going. The framing is the right shape: the source adapter handles backfill, the plugin handles the live side. It isn't mergeable yet, though:
- Step 3 breaks OpenCode. The key is
plugin, notplugins. OpenCode's schema hasadditionalProperties: false, so the config as written fails validation. Your own README already usesplugin. - #1484 hasn't landed. The doc says MemPalace "ships"
mempalace.sources.opencode. It doesn't yet. Either wait for #1484 or say "once #1484 merges". - The doc describes v1.3, not 2.1.0. It still says flat export, no wings,
--mode convosonly, and "every turn is saved as a drawer". 2.1.0 has per-project wings,--extract general, checkpoint and pre-compaction prompts,saveInterval, and a backfill env var. Please describe what the pinned version actually does. - Verbatim storage.
--extract generaldrops any paragraph the heuristic doesn't classify, and only text parts are exported. That breaks "every word, verbatim". Please use the defaultexchangeextraction for the transcript. The checkpoints are fine as an extra layer on top of that. - Capture gap. The checkpoint sync fires 500ms after the user message, which is usually mid-reply. It then advances the cursor past the assistant message, so the rest of that reply is never captured. Please sync on
session.idleonly, or track completion per message. - Exit path. The synchronous exit save can block for 30s per query plus 60s per wing. It overrides the default SIGINT/SIGTERM exit, and it ignores
miningLock, so it can start a second mine while one is running. Exit needs to stay fast and never race the lock. - Privacy. Transcripts are written world-readable to a fixed
/tmp/oc-sessions, and a fixed/tmp/oc-plugin-query.pygets executed. Please use a per-user 0700 directory under~/.mempalace, ormkdtemp. - Windows. The frontmatter lists
win32, but the binary is hardcoded to~/.local/bin/mempalaceand the venv lookup tobin/python3. Either resolve the binary from PATH (or an env override), or dropwin32and add the Windows note I asked for. - Docs hygiene.
- Pin the npm version in the doc.
- Use one tool-name convention (
mempalace_mempalace_*) throughout. - Link
integrations/shared/recall-protocol.mdinstead of a "CRITICAL / MUST every response" AGENTS.md. - Don't tell users to create
~/.config/opencode/AGENTS.md; that overwrites the shared-brain rules. - Drop or re-measure the "<500ms" claim. A cold
mempalace searchtook about 20s here. - Set
version:to the current release.
Open question on our side: does a community npm package belong behind an install block in an official integrations/ skill, or as a website/guide/opencode.md page that links to it clearly as third-party? I lean toward the guide page.
|
Thanks for the thorough review, @igorls — this is exactly the kind of pass this needed. Addressed point by point (doc updated on this branch, plugin fixes landed on
On the open question (skill vs guide page): happy either way — a Re-review welcome whenever suits. |
|
Small update, no action needed — just keeping the thread accurate while you have bigger fish to fry. The plugin is now at 2.5.1 on npm (all dogfooded daily). Since my last note: TUI toasts for mines, checkpoints and MCP calls (opt-out), a read-only The SKILL.md on this branch describes the 2.2.0 behavior; I'll sync it to latest whenever you signal the direction is still good — didn't want to churn the branch on every release. @igorls @williamblair333 — whenever you have a moment, no rush at all. |
|
Brief update, no rush at all — just keeping the thread honest. The plugin is now at 2.8.1 on npm. Notable since my last note: message-level export dedup (each message is archived exactly once — this, not context size, was the real fix for near-duplicate recall), per-wing sync cursors (one slow project can no longer stall the others), and a community-reported ENOBUFS fix that made exports resilient for users with very large sessions. Adoption is growing on its own: ~2,000 npm downloads/week and daily dogfooding on my trading-bot setup, so this feels like a useful integration to have available for OpenCode users. I know review capacity is finite — whenever you get a chance to re-check the branch (it's synced to the latest behavior), I'm happy to address anything that remains. Thanks! P.S. The "Changes requested" label on a PR clears automatically as soon as a reviewer re-reviews and approves — there's no way (and no need) for me to remove it myself; it just needs the next review pass. |
|
Update from the plugin side, no action needed — just keeping the thread current. OpenCode 2 landed and the plugin now supports it. v2 changed both the plugin contract and the on-disk session layout, so two things were needed:
The bug worth reporting to other integrations. The sync cursor used to advance only after a successful mine. The export is cheap and idempotent; the mine is expensive and fails for reasons outside the plugin's control (palace lock held elsewhere, killed at exit, OOM). So a mine that never finished pinned the cursor forever, every later export re-cut its window from the same stale point, and because the session kept growing each file was a superset of the previous one. One long-running session produced 691 overlapping files; mining them all multiplied every message by up to 691 — 628k drawers, 5 GB, with no error anywhere. The cursor now advances on write, so windows are disjoint by construction and the pending file is simply the queue. Full write-up in the README: https://github.com/geco/opencode-mempalace-persistence#the-cursor-means-exported-not-mined One caveat, stated plainly: v2's server runtime has no toast surface, so the plugin's TUI notifications are silent there today. v2 does expose a TUI-side entry point with Adoption is steady at ~2,700 npm downloads in the last 30 days, on top of the daily dogfooding on my trading-bot setup. Still happy to rebase the skill file onto whatever the current MemPalace behaviour is whenever you have a moment to look at it. |
This PR adds an integration guide for using MemPalace with OpenCode via the
opencode-mempalace-persistenceplugin.What
integrations/opencode/SKILL.md— step-by-step setup for:Why
OpenCode is a popular open-source AI coding tool. This integration makes MemPalace available to its users with zero configuration — install the plugin, set up MCP, and every conversation is automatically saved to the palace.
Features documented
The integration covers:
Links